Skip to content

[Test] Extend local soak baseline to SSE streaming responses - #3447

Open
xuuuge wants to merge 8 commits into
vllm-project:mainfrom
xuuuge:codex/issue-3340-sse-soak
Open

[Test] Extend local soak baseline to SSE streaming responses#3447
xuuuge wants to merge 8 commits into
vllm-project:mainfrom
xuuuge:codex/issue-3340-sse-soak

Conversation

@xuuuge

@xuuuge xuuuge commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Closes #3340

Purpose

Extend the existing local soak harness with a launch-selectable SSE response mode while preserving the buffered baseline.

  • send OpenAI Chat Completions requests with stream: true and consume the full event stream
  • shape valid mock-backend SSE into repeatable first-frame delay, inter-frame interval, and content-frame-count controls
  • keep the router on Envoy's STREAMED response path for the response lifetime
  • record response-mode: streaming in JSON, benchstat, and run-environment artifacts
  • document buffered and streaming commands side by side without adding checked-in config
  • tolerate current Envoy templates that no longer contain an ext_authz filter when deriving the local soak config

Test Plan

  • make agent-ci-gate ENV=cpu AGENT_BASE_REF=origin/main
  • make soak-test
  • python -m pytest bench/test_openai_fault_proxy.py -q
  • make build-router in a clean temporary worktree whose path has no spaces
  • run make soak-local SOAK_STREAMING=1 SOAK_ARGS=-quick

Test Result

  • PASS: full agent-ci-gate, including ShellCheck, Black, Ruff, markdown lint, supply-chain scan, Go structural lint, architecture checks, build-e2e, 116 workflow/CI tests, and 29 agent-harness tests
  • PASS: make soak-test (build, vet, and Go soak unit tests)
  • PASS: fault-proxy suite, 7 tests in 3.81s; the SSE test checks first-frame delay, per-frame pacing, frame count, terminal event, and [DONE]
  • PASS: full router build
  • PASS: end-to-end streaming quick-soak at concurrency 100 with streaming: true, zero scrape errors, complete JSON/heap/timeseries/bench artifacts, and 5,715/5,787 measured requests succeeded (98.76%).

Semantic Router PR Checklist

Signed-off-by: xuuuge <xuuuge@users.noreply.github.com>
@netlify

netlify Bot commented Sep 3, 2026

Copy link
Copy Markdown

Deploy Preview for vllm-semantic-router ready!

Name Link
🔨 Latest commit 323125b
🔍 Latest deploy log https://app.netlify.com/projects/vllm-semantic-router/deploys/6a9eed50c4c66c00088c0823
😎 Deploy Preview https://deploy-preview-3447--vllm-semantic-router.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

Signed-off-by: xuuuge <xuuuge@users.noreply.github.com>
Signed-off-by: xuuuge <xuuuge@users.noreply.github.com>
@github-actions github-actions Bot added pr/needs-author Waiting for author changes or response. wg/data-plane-networking Owned by the Data Plane and Networking Workgroup. labels Sep 3, 2026
@xuuuge
xuuuge marked this pull request as ready for review September 3, 2026 22:31
@github-actions github-actions Bot added pr/needs-rebase Needs rebase or conflict resolution. and removed pr/needs-author Waiting for author changes or response. labels Sep 3, 2026

@FAUST-BENCHOU FAUST-BENCHOU left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing to consider is avoiding buffering the entire upstream SSE response before forwarding.

Currently response.read() consumes the full stream before send_sse() starts writing chunks, which means the proxy does not preserve the upstream first-frame latency and inter-frame timing. Forwarding frames as they arrive would make the soak test closer to a real streaming path.

@Xunzhuo Xunzhuo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The streaming fixture and synthetic merge look clean, but exact-head CI currently covers only lint/build: neither make soak-test nor pytest bench/test_openai_fault_proxy.py is executed. Please add exact-head CI evidence for both behavior paths before approval.

Avoid consuming the upstream stream before forwarding its first event. Keep optional frame shaping incremental and preserve terminal and usage events. Add gated-upstream regression coverage and run both soak and fault-proxy behavior suites on the exact PR head in core quality CI.

Signed-off-by: xuuuge <xuuuge@users.noreply.github.com>
@xuuuge
xuuuge requested a review from rootfs as a code owner September 6, 2026 16:39
@xuuuge

xuuuge commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

@FAUST-BENCHOU @Xunzhuo addressed both review comments in 632410e.

  • SSE responses now forward and flush complete upstream events as they arrive, without response.read() buffering the full stream. Optional frame-count shaping is incremental and retains only the requested number of events for replay; terminal, usage, and [DONE] events retain their order.
  • Added gated-upstream regression tests that withhold each subsequent event until the client receives the preceding one, covering LF/CRLF framing, fragmented HTTP chunks, and shaped/unshaped streaming. The regression fails against the original implementation and passes with this fix.
  • Added Core quality / Soak behavior (PR head) to the existing PR workflow. It explicitly checks out github.event.pull_request.head.sha, records the tested SHA, and runs both make soak-test and python -m pytest bench/test_openai_fault_proxy.py -q.

Local validation passed: make soak-test, all 12 proxy tests, and make agent-ci-gate (including lint, structure/security checks, 116 workflow/CI tests, and 29 harness tests).

The CI run for this exact head has been triggered but is currently queued before the initial change-classification job. Hosted behavior-test evidence is therefore still pending; the local passes above are not a claim that CI has completed.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 33.54%. Comparing base (aff932e) to head (632410e).
⚠️ Report is 228 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3447      +/-   ##
==========================================
- Coverage   33.93%   33.54%   -0.39%     
==========================================
  Files          20       20              
  Lines        2959     2942      -17     
==========================================
- Hits         1004      987      -17     
  Misses       1849     1849              
  Partials      106      106              
Flag Coverage Δ
operator 33.54% <ø> (-0.39%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Xunzhuo Xunzhuo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks—the exact-head soak job now covers both behavior paths, and the proxy forwards each SSE event before the upstream can continue while preserving terminal and usage events. I do not see a remaining code blocker; please update onto current main and let the required checks finish for the approval pass.

@github-actions github-actions Bot added pr/blocked Blocked on a named decision, dependency, or required check. and removed pr/needs-rebase Needs rebase or conflict resolution. labels Sep 6, 2026
Signed-off-by: xuuuge <xuuuge@users.noreply.github.com>
The anthropic-shim PR lane failed before executing tests because the model-downloader init container exceeded its 512Mi limit and was OOMKilled. Raise the download memory request to 512Mi and limit to 2Gi, keeping the change scoped to the setup container.

Signed-off-by: xuuuge <xuuuge@users.noreply.github.com>
@xuuuge

xuuuge commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

@Xunzhuo @FAUST-BENCHOU updated the PR onto current main (eb678935) in merge commit 02193d1. Could you take another review pass once the new required checks are green?

The previous exact-head soak job passed both behavior suites. The remaining PR-gate failure came from Anthropic-shim setup: its model-downloader init container was OOMKilled under a 512 MiB limit, before the E2E tests ran. Commit e6ebe40 raises only that container's memory request to 512 MiB and limit to 2 GiB to provide download headroom.

Local validation after updating main passed: make soak-test, all 12 fault-proxy tests, and the full changed-file make agent-ci-gate (including E2E build, lint/security/structure checks, 132 workflow tests, and 29 harness tests). The downloader manifest also passes YAML and structural checks. The new CI run will validate the Anthropic setup fix; that hosted result is still pending.

GitHub did not permit this account to formally re-request reviews through the API, so I am requesting the approval pass here.

@github-actions github-actions Bot added pr/needs-review Ready for reviewer attention. pr/needs-rebase Needs rebase or conflict resolution. and removed pr/blocked Blocked on a named decision, dependency, or required check. pr/needs-review Ready for reviewer attention. pr/needs-rebase Needs rebase or conflict resolution. labels Sep 7, 2026
@github-actions github-actions Bot added pr/needs-review Ready for reviewer attention. and removed pr/needs-rebase Needs rebase or conflict resolution. labels Sep 7, 2026
@github-actions github-actions Bot added pr/needs-rebase Needs rebase or conflict resolution. and removed pr/needs-review Ready for reviewer attention. labels Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr/needs-rebase Needs rebase or conflict resolution. wg/data-plane-networking Owned by the Data Plane and Networking Workgroup.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Extend the local soak baseline to SSE/streaming responses

5 participants